feat: add install-info command and buildInstallSnippet API - #18
Conversation
Re-checking the originally planned Cursor "fixes" against the vendored plugin.schema.json / marketplace.schema.json (this repo's own conformance oracle, already passing against the current shape) showed they were wrong: - displayName/category/tags ARE valid plugin.json fields per the schema (additionalProperties: false, and they're explicitly listed) — not marketplace-entry-only fields as previously assumed. - Marketplace `owner` is genuinely optional (required: ["name", "plugins"] does not include it) — not required as previously assumed. Moving category/tags to the marketplace entry would have been actively wrong: entries only allow name/source/description (additionalProperties: false). None of that is changed here. What this commit actually does: - Migrates cursor onto PluginTargetDefinition, preserving every existing field and behavior (verified by the vendored-schema conformance test staying green). - Fixes one genuine, low-risk issue: the manifest builder's own hardcoded default-components list could diverge from this target's actual defaultComponents (components.ts). Replaced both with one list of schema-valid pointer fields, checked directly against the plugin's real resolved componentDirs — eliminates the divergence risk with no observable behavior change (confirmed via a new test exercising the one case that could have differed: an explicit `components: [...]` override). - Ports update-check's hook-injection into the new shared engine (src/targets/engine.ts), which previously only existed in the legacy emitCursor/emitClaude path — migrating cursor without this would have silently dropped update-check support. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
buildPluginManifest used the target-level `version` param directly instead of `pluginConfig.version ?? version`, silently dropping a per-plugin version override — a real regression from the pre-migration behavior, caught by porting the equivalent test from the claude migration (no test previously covered this for cursor specifically). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ports emitClaude/validateClaude into src/targets/claude.ts as a PluginTargetDefinition, verified directly against `claude plugin validate --strict`: a minimal manifest with only `name` fails that check, confirming the existing version/description/author.name requirements already match the real CLI rather than over-constraining it. Applies the per-plugin version override in buildPluginManifest (pluginConfig.version ?? version) up front, matching the identical fix just made to cursor's copy of this pattern.
Ports emitCodex/validateCodex into src/targets/codex.ts as a PluginTargetDefinition, correcting the plugin format's real shape — re-verified directly against developers.openai.com/codex/plugins/build (fetched twice, independently, for consistency) since Codex has no CLI validator or vendored schema to check against: - plugin.json requires only "name"; version/description/author etc. are optional. The previous validator wrongly required version and description. - Every marketplace entry needs policy.installation, policy.authentication, and category — previously unvalidated, so an incomplete entry shipped silently. pluginpack can't infer these, so the base entry stays guess-free and validateOutput now errors clearly when an author never supplies them via the per-plugin `entry` passthrough (already how the existing conformance fixture supplies them). - A marketplace entry's source is a bare string only for local plugins (the only shape pluginpack itself ever emits); url/git-subdir/npm sources are structured objects with an inner "source" discriminator. validateMarketplaceEntry now accepts either shape instead of the previously shared, string-only validator. - plugin.json now declares a `hooks` pointer when hooks/ is present, matching skills/mcpServers (previously only skills/mcpServers were declared, so hooks were emitted but never referenced). Updates CONFORMANCE.md's Codex section, which had pinned a stale, bare-string-only shape from an earlier doc retrieval.
…tion Every migrated target's validateOutput calls validateHooksShape (added in the registry scaffold), which only checked that hooks.json has a "hooks" object — narrower than the legacy per-target validateHooks it replaced, which also required each event's entries to be an array, rejected empty "command" strings, and errored if a command referenced the generated update-check script without that script actually being present. None of that depth had a regression test, so the narrowing was silent. Ports the full check into validateHooksShape once, so every target that already calls it (all 5, post-migration) regains it for free instead of needing the fix repeated per target file.
…argets are migrated All 5 targets (copilot, antigravity, cursor, claude, codex) now have a PluginTargetDefinition in src/targets/registry.ts, so the legacy fallback path adapters.ts existed for is dead: - Deletes src/targets.ts and src/validate.ts entirely (their only consumer was adapters.ts's legacyAdapters map). - Moves withRootFiles into src/targets/engine.ts, next to the artifact helper it depends on. - Tightens the registry's type from Partial<Record<TargetName, ...>> to Record<TargetName, ...> now that every target has an entry — a new TargetName won't build until it has a registry entry, the same exhaustiveness guarantee the deleted legacyAdapters map used to provide. - Simplifies adapters.ts to a thin emitTarget/validateOutput wrapper around the registry + engine, dropping the now-pointless TargetAdapter/adapters indirection that existed only to switch between legacy and registry per target. - Removes targetDefaultComponents/resolveTargetComponents from components.ts (superseded by each target's own defaultComponents). - Updates CLAUDE.md's Architecture/Targets sections and a couple of stale doc-comment references to match.
Cross-references the validateHooksShape fix from the prior commit — what it checks and where it's shared from, for the conformance doc's own "what's actually verified and how" mandate.
Adds the install-snippet feature designed alongside the target registry migration: every PluginTargetDefinition already carries an installSnippet (populated as each target migrated), so this wires it up to a public API and CLI command rather than introducing new per-target data. - src/install-snippet.ts: buildInstallSnippet, getInstallSnippetCitation, getSupportedInstallTargets, getUnsupportedInstallTargets — thin wrappers around each target's registry entry. - New `pluginpack install-info [--target <t>] [--json]` CLI command, defaulting to every configured target (matching `build`'s pattern). Resolves each plugin's path via that target's own resolvePluginPath for accuracy (antigravity's snippet is the one that consumes it). - targets.<name>.repository config field (falls back to metadata.repository, mirroring updateCheck.repository's existing fallback) — "which repo does this target's output live in." - Exported from the package entry for programmatic use. - README: new "Install Snippet" section + Configuration Reference row + Programmatic API table rows. CONFORMANCE.md: "Install-snippet facts" table with per-target doc citations. Tests: tests/install-snippet.test.ts locks in the exact snippet text per target; tests/conformance.test.ts adds CLI end-to-end coverage (default/--target/--json/missing-repository error) via the real built binary.
0ec21f5 to
f49ddc4
Compare
| target: TargetName, | ||
| params: InstallParams, | ||
| ): InstallSnippet { | ||
| const definition = registry[target].installSnippet; |
There was a problem hiding this comment.
Issue: Potential TypeError: buildInstallSnippet indexes target registry without handling partial registry entries. If registry[target] is undefined (e.g., targets not yet migrated to the registry), accessing .installSnippet will throw at runtime when CLI install-info or library callers pass such a target.
Suggested fix: Mirror the adapters' partial-registry pattern: guard registry[target] and fall back to a legacy install snippet definition or return a structured userConfigurable:false result with a clear reason. For example: const def = registry[target]?.installSnippet; if (!def || !def.userConfigurable || !def.build) return { userConfigurable: false, reason: def?.unsupportedReason ?? ${target} has no install command or URL. }; otherwise return { userConfigurable: true, ...def.build(params) }. Alternatively, ensure all TargetName variants are present in the registry before using it here.
🔧 Tag @ glean-for-engineering to fix or click here to fix in Glean
💬 Help us improve! Was this comment helpful? React with 👍 or 👎
# ------------------------ >8 ------------------------ # Do not modify or remove the line above. # Everything below it will be ignored. # # Conflicts: # CONFORMANCE.md
Summary
Implements the install-snippet feature planned alongside the target registry migration — every
PluginTargetDefinitionalready carries aninstallSnippet(populated as each target migrated in #14/#15/#16), so this wires it up to a public API and CLI command rather than introducing new per-target data.src/install-snippet.ts:buildInstallSnippet,getInstallSnippetCitation,getSupportedInstallTargets,getUnsupportedInstallTargets— thin wrappers around each target's registry entry.pluginpack install-info [--target <t>] [--json]CLI command, defaulting to every configured target (matchingbuild's pattern). Resolves each plugin's path via that target's ownresolvePluginPathfor accuracy — antigravity's snippet is the one that actually consumespluginPath.targets.<name>.repositoryconfig field (falls back tometadata.repository, mirroringupdateCheck.repository's existing fallback) — "which repo does this target's output live in."buildInstallSnippet,getSupportedInstallTargets,getUnsupportedInstallTargets, plusCitation/InstallParams/InstallSnippettypes) for programmatic use.CONFORMANCE.md: an "Install-snippet facts" table with each target's doc citation.Test plan
npm run typechecknpm run lintnpm test(76/76 —tests/install-snippet.test.tslocks in the exact snippet text per target;tests/conformance.test.tsadds CLI end-to-end coverage — default,--target,--json, missing-repository error — via the real built binary)npm run buildnode dist/cli.js docs --checkinstall-infoagainst a 5-target fixture in/tmp(all 5 snippets print correctly;--target/--json/missing-repo error all behave as expected)Stacking
Based on
refactor/delete-legacy-target-adapters(#17). Merge in order: #14 → #15 → #16 → #17 → this.